Checkmarx One – Scan Summary & Details – ead6ce58-7464-4c1e-9683-47bab2532cda

Great job! No new security vulnerabilities introduced in this pull request

Claude finished @bw-ghapp[bot]'s task —— View job

Code Review Complete

Tasks

• Gather context and understand PR changes
• Perform comprehensive code review using bitwarden-code-reviewer agent
• Analyze findings and prepare feedback
• Post final review

Review Summary

I've completed a thorough review of this API bindings update (commit 8a67aafbe5e01a738b479ebb82185fbc947866f8). This PR updates auto-generated API clients and includes necessary integration code changes.

Critical Finding

🔴 Security Vulnerability in Self-Hosted Billing API

The newly added self_hosted_billing_api.rs contains endpoints that transmit sensitive cryptographic keys (private_key, public_key) as URL query parameters (lines 337-339):

if let Some(ref param_value) = private_key {
    local_var_req_builder =
        local_var_req_builder.query(&[("privateKey", &param_value.to_string())]);
}

Why this is critical:

• URLs with query parameters are logged by web servers, proxies, and load balancers
• Query parameters appear in browser history unencrypted
• URLs can leak via Referer headers to third-party sites
• Query parameters are visible in network traces

Note: As @dani-garcia mentioned, these endpoints are incorrectly generated and won't work. However, this should be explicitly documented to prevent accidental usage.

Recommendation:

1. Add documentation warning that self_hosted_billing_api::get_metadata() should NOT be used
2. File an issue with the server team to fix the OpenAPI spec (move sensitive params to request body)
3. Consider adding a compile-time deprecation warning

Other Findings

✅ Acceptable Breaking Change

• OrganizationUpdateRequestModel::new() constructor signature changed from new(name: String, billing_email: String) to new()
• Required fields became optional
• Breaking change detection passed for TypeScript clients
• No internal SDK usages found
• Status: Acceptable (auto-generated code, validated by automation)

ℹ️ KDF Settings Migration Question

• New kdf_settings field added to PasswordPreloginResponseModel
• Integration code in prelogin.rs still uses individual fields (kdf, kdf_iterations, etc.)
• Both formats coexist in the response (appears to be transitional API design)
• Current implementation works correctly
• Question: Is there a migration plan to use kdf_settings instead of individual fields?

📊 Test Coverage Gaps (Non-blocking)

• Codecov reports 4 lines missing coverage in prelogin.rs and policy.rs
• Suggestion: Add unit tests for:
  • parse_prelogin with both PBKDF2 and Argon2id KDF types
  • Policy type conversion including new BlockClaimedDomainAccountCreation variant
  • Edge cases (missing KDF type, invalid iterations)

✅ Already Fixed

• Prelogin model rename (PreloginRequestModel → PasswordPreloginRequestModel) properly integrated in commit 5635837
• New policy type BlockClaimedDomainAccountCreation correctly mapped
• New event types properly added

Architecture Compliance

✅ Per CLAUDE.md: "DO NOT edit" auto-generated API clients - this PR correctly contains only auto-generated changes plus necessary integration updates

✅ Integration code follows established patterns

✅ No security-sensitive data logged in error paths (besides the upstream API design issue)

Recommendation

Request Changes - The security vulnerability in the self-hosted billing API must be addressed before these endpoints can be safely used. Since these endpoints are incorrectly generated and non-functional (per maintainer comment), I recommend:

1. Adding inline documentation/comments warning against usage of the get_metadata endpoint
2. Creating a tracking issue for the server team to fix the OpenAPI specification
3. Otherwise, the API binding updates and integration changes are correct

The rest of the changes are acceptable as auto-generated API bindings with proper integration updates.

🔍 SDK Breaking Change Detection Results

SDK Version: sdlc/api-update (e7cb182)
Completed: 2025-12-08 15:39:20 UTC
Total Time: 249s

Breaking change detection completed. View SDK workflow

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.73%. Comparing base (5c178be) to head (e7cb182).
⚠️ Report is 2 commits behind head on main.

@@            Coverage Diff             @@
##             main     #578      +/-   ##
==========================================
- Coverage   79.74%   79.73%   -0.01%     
==========================================
  Files         304      304              
  Lines       32757    32758       +1     
==========================================
  Hits        26121    26121              
- Misses       6636     6637       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.